Added Health Check Module for Ktor#5498
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Ktor server plugin "HealthCheck" with a configuration DSL for readiness and liveness endpoints, concurrent suspendable named checks, GET-only call interception, JSON responses with per-check statuses and overall 200/503 mapping; includes README, build/settings, implementation, and tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt`:
- Around line 75-80: The evaluateCheck function currently catches all Exceptions
and returns CheckStatus.DOWN with the raw cause.message which incorrectly
converts cancellations to DOWN and leaks backend error text; update
evaluateCheck to rethrow CancellationException (and other Cancellation-related
exceptions) instead of catching them, and when returning a DOWN result sanitize
the reason by using a generic message like "check failed" unless a configured
diagnostics flag (e.g., detailedDiagnosticsEnabled) is true, in which case
include the real cause message; reference the evaluateCheck function,
NamedCheck.check invocation, and CheckResult construction to locate where to
change the catch behavior and how to build the sanitized CheckResult.
- Around line 108-112: The escapeJson() implementation on String currently only
handles quotes, backslashes, and \n/\r/\t, leaving other control characters
(U+0000..U+001F) like \b and \f unescaped and producing invalid JSON; update
escapeJson() to iterate characters (or use a regex) and: map backspace and
formfeed to \"\\b\" and \"\\f\" respectively, keep the existing mappings for
'\\', '\"', '\\n', '\\r', '\\t', and for any other code point <= 0x1F produce a
\\u00XX hex escape; ensure the function returns the correctly escaped JSON
string for use in HealthCheck output.
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 27-77: This change introduced new public API types
(HealthCheckConfig, HealthCheckBuilder and the public HealthCheck plugin entry
point) but the module’s ABI signatures under the module's /api/ directory
weren’t updated; run the module's Gradle ABI update (./gradlew
:<module-name>:updateLegacyAbi) to regenerate the API signature files, verify
the new entries for HealthCheckConfig, HealthCheckBuilder and the HealthCheck
plugin are present in the /api/ output, and commit those generated signature
files so the legacy ABI checks include the new public DSL surface.
- Around line 39-53: Both readiness(path: String, block: HealthCheckBuilder.()
-> Unit) and liveness(path: String, block: HealthCheckBuilder.() -> Unit)
blindly append a new HealthCheckEndpoint to endpoints, allowing duplicate
normalized paths which later get ignored; change both methods to normalize the
path (use path.ensureLeadingSlash()), check endpoints.any { it.path ==
normalized } and require that it is false, e.g. require(!endpoints.any { it.path
== normalized }) { "Duplicate health-check endpoint configured: $normalized
(already registered)" }, then proceed to create and append the
HealthCheckEndpoint using HealthCheckBuilder().apply(block).checks.toList().
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt`:
- Around line 112-126: Update the test function `only GET requests are handled`
to assert the explicit response for POST /health instead of only checking that a
substring is absent: after installing `HealthCheck` with `liveness("/health")`
call `client.post("/health")` and assert the concrete expected status code
(e.g., HttpStatusCode.MethodNotAllowed or the plugin's actual response) and the
exact response body text you expect for POST, replacing the current
contains/false check so the test fails on regressions that change POST behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 77c0e88c-5838-4055-bd82-c218b4c6f0a7
📒 Files selected for processing (6)
ktor-server/ktor-server-plugins/ktor-server-health-check/README.mdktor-server/ktor-server-plugins/ktor-server-health-check/build.gradle.ktsktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.ktsettings.gradle.kts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt (1)
75-80:⚠️ Potential issue | 🟠 MajorDon't turn cancellation into
DOWN, and don't expose raw failure text.Line 79 catches
CancellationExceptionviaException, so shutdown/request cancellation is reported as a normal health failure. Line 80 also returns raw backend messages from a typically public probe endpoint. Rethrow cancellation and return a sanitized failure string instead.🔒 Proposed fix
private suspend fun evaluateCheck(namedCheck: NamedCheck): CheckResult = try { val healthy = namedCheck.check() CheckResult(namedCheck.name, if (healthy) CheckStatus.UP else CheckStatus.DOWN) + } catch (cause: CancellationException) { + throw cause } catch (cause: Exception) { - CheckResult(namedCheck.name, CheckStatus.DOWN, cause.message) + CheckResult(namedCheck.name, CheckStatus.DOWN, "Health check failed") }Based on learnings: "In Ktor,
io.ktor.utils.io.CancellationExceptionis a typealias forkotlinx.coroutines.CancellationException, which is a supertype ofJobCancellationException. Therefore, checkinge !is io.ktor.utils.io.CancellationExceptionis sufficient to exclude all coroutine cancellation exceptions."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt` around lines 75 - 80, In evaluateCheck, avoid turning coroutine cancellations into DOWN and stop leaking raw error text: change the catch in evaluateCheck(namedCheck: NamedCheck) to rethrow cancellation exceptions (check for io.ktor.utils.io.CancellationException or kotlinx.coroutines.CancellationException) and for other exceptions return CheckResult(namedCheck.name, CheckStatus.DOWN, "<unavailable>" or a fixed sanitized message) instead of cause.message; keep references to NamedCheck, CheckResult and CheckStatus so the handler location is clear.
🧹 Nitpick comments (1)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt (1)
96-97: Reject blank health-check names when registering a check.
check("", ...)currently succeeds and produces ambiguous"name"fields in the JSON payload and logs. Validatename.isNotBlank()before storing it.🛡️ Proposed fix
public fun check(name: String, block: suspend () -> Boolean) { + require(name.isNotBlank()) { + "Health check name must not be blank (received length=${name.length})" + } checks += NamedCheck(name, block) }As per coding guidelines: "Use
require(...)for argument validation" and "Make error messages actionable by including the problematic value and context".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt` around lines 96 - 97, The check(name: String, block: suspend () -> Boolean) function currently allows blank names; update it to validate the argument with require(name.isNotBlank()) before adding NamedCheck to checks, and make the require message actionable by including the invalid value and context (e.g. "health check name must not be blank: '<value>'") so callers get a clear error; adjust any tests or callers if they relied on blank names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 27-40: Remove the accidental nested duplicate class declaration
and the extra closing braces: delete the second "class HealthCheckConfig" block
and its duplicate "internal val endpoints =
mutableListOf<HealthCheckEndpoint>()" so there is a single HealthCheckConfig
class containing the existing methods (e.g., readiness endpoint configuration
methods). Ensure the remaining braces are balanced for the single
HealthCheckConfig declaration and that only one endpoints property exists.
---
Duplicate comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt`:
- Around line 75-80: In evaluateCheck, avoid turning coroutine cancellations
into DOWN and stop leaking raw error text: change the catch in
evaluateCheck(namedCheck: NamedCheck) to rethrow cancellation exceptions (check
for io.ktor.utils.io.CancellationException or
kotlinx.coroutines.CancellationException) and for other exceptions return
CheckResult(namedCheck.name, CheckStatus.DOWN, "<unavailable>" or a fixed
sanitized message) instead of cause.message; keep references to NamedCheck,
CheckResult and CheckStatus so the handler location is clear.
---
Nitpick comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 96-97: The check(name: String, block: suspend () -> Boolean)
function currently allows blank names; update it to validate the argument with
require(name.isNotBlank()) before adding NamedCheck to checks, and make the
require message actionable by including the invalid value and context (e.g.
"health check name must not be blank: '<value>'") so callers get a clear error;
adjust any tests or callers if they relied on blank names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e5c99178-a091-42f3-bd4f-fe93f5b61359
📒 Files selected for processing (2)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt (1)
180-191: Consider adding a test for duplicate endpoint path rejection.The configuration validates against duplicate paths with
require(), but there's no test verifying this behavior throws an exception.✅ Proposed test
`@Test` fun `duplicate endpoint path throws exception`() = testApplication { val exception = assertFailsWith<IllegalArgumentException> { install(HealthCheck) { liveness("/health") { check("ok") { true } } readiness("/health") { check("db") { true } } } } assertContains(exception.message.orEmpty(), "/health") assertContains(exception.message.orEmpty(), "already configured") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt` around lines 180 - 191, Add a unit test that verifies the HealthCheck configuration rejects duplicate paths by asserting an IllegalArgumentException is thrown when installing HealthCheck with the same path twice; in the test (e.g., name it `duplicate endpoint path throws exception`) call install(HealthCheck) and configure liveness("/health") { check("ok") { true } } and readiness("/health") { check("db") { true } } inside assertFailsWith<IllegalArgumentException> and assert that the exception.message contains "/health" and "already configured" to ensure the require() validation in the HealthCheck setup is exercised.ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt (1)
83-85: Consider validating check names to prevent duplicates and empty names.The
check()method accepts any string as a name without validation. Empty/blank names or duplicate check names within the same endpoint would produce confusing health responses.🛡️ Proposed validation
public fun check(name: String, block: suspend () -> Boolean) { + require(name.isNotBlank()) { "Health check name must not be blank" } + require(checks.none { it.name == name }) { + "Health check '$name' is already defined in this endpoint" + } checks += NamedCheck(name, block) }As per coding guidelines: "Use
require(...)for argument validation" and "Make error messages actionable by including the problematic value and context".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt` around lines 83 - 85, The check(name: String, block: suspend () -> Boolean) method in HealthCheckConfig accepts any name; add argument validation using require(...) to ensure name.isNotBlank() and throw a clear message including the provided name and context (e.g. "check name must not be blank: '<name>'"); also ensure uniqueness by checking the existing checks collection (checks) for an existing NamedCheck with the same name and require it not to exist, with an actionable message like "duplicate check name for HealthCheckConfig: '<name>'"; update the check(...) function to perform these two require(...) validations before adding NamedCheck(name, block).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt`:
- Around line 25-31: The test contains a duplicate assertion asserting
"\"status\":\"UP\"" twice in the client.get("/health") response block; remove
the second assertContains("\"status\":\"UP\"") (the duplicate in the response
handling block in HealthCheckTest.kt) so the test only checks overall status
once and the check name once (leave the assertions for response.status, body
contains "\"status\":\"UP\"" and "\"name\":\"alive\"").
---
Nitpick comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 83-85: The check(name: String, block: suspend () -> Boolean)
method in HealthCheckConfig accepts any name; add argument validation using
require(...) to ensure name.isNotBlank() and throw a clear message including the
provided name and context (e.g. "check name must not be blank: '<name>'"); also
ensure uniqueness by checking the existing checks collection (checks) for an
existing NamedCheck with the same name and require it not to exist, with an
actionable message like "duplicate check name for HealthCheckConfig: '<name>'";
update the check(...) function to perform these two require(...) validations
before adding NamedCheck(name, block).
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt`:
- Around line 180-191: Add a unit test that verifies the HealthCheck
configuration rejects duplicate paths by asserting an IllegalArgumentException
is thrown when installing HealthCheck with the same path twice; in the test
(e.g., name it `duplicate endpoint path throws exception`) call
install(HealthCheck) and configure liveness("/health") { check("ok") { true } }
and readiness("/health") { check("db") { true } } inside
assertFailsWith<IllegalArgumentException> and assert that the exception.message
contains "/health" and "already configured" to ensure the require() validation
in the HealthCheck setup is exercised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38f4eb74-6a29-498e-9fc9-589cd19fa299
📒 Files selected for processing (3)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt
c2a202b to
fe974f9
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt (1)
31-37: Validate blankpathand blank checknameduring configuration.
readiness/livenesscurrently allow""(normalized to/), andcheck()allows empty names. Validating both early makes misconfiguration obvious.♻️ Proposed fix
private fun addEndpoint(path: String, block: HealthCheckBuilder.() -> Unit) { + require(path.isNotBlank()) { + "Health check endpoint path must not be blank (received: '$path')" + } val normalizedPath = path.ensureLeadingSlash() require(endpoints.none { it.path == normalizedPath }) { "Health check endpoint path '$normalizedPath' is already configured" } endpoints += HealthCheckEndpoint(normalizedPath, HealthCheckBuilder().apply(block).checks.toList()) } @@ public fun check(name: String, block: suspend () -> Boolean) { + require(name.isNotBlank()) { + "Health check name must not be blank (received: '$name')" + } checks += NamedCheck(name, block) }As per coding guidelines: "Use
require(...)for argument validation" and "Make error messages actionable by including the problematic value and context".Also applies to: 83-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt` around lines 31 - 37, Reject blank endpoint paths and blank check names early: in addEndpoint (and the other endpoint-adding overload around lines 83-85) add a require that the normalizedPath is not "/" (or that the original path is not blank) with an actionable message including the offending value; and in HealthCheckBuilder.check validate the provided name with require(name.isNotBlank()) and an error message that includes the empty name/context. Use require(...) for these argument checks and reference HealthCheckEndpoint/HealthCheckBuilder.addEndpoint and HealthCheckBuilder.check so the validation is added in the right methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 31-37: Reject blank endpoint paths and blank check names early: in
addEndpoint (and the other endpoint-adding overload around lines 83-85) add a
require that the normalizedPath is not "/" (or that the original path is not
blank) with an actionable message including the offending value; and in
HealthCheckBuilder.check validate the provided name with
require(name.isNotBlank()) and an error message that includes the empty
name/context. Use require(...) for these argument checks and reference
HealthCheckEndpoint/HealthCheckBuilder.addEndpoint and HealthCheckBuilder.check
so the validation is added in the right methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5aaf6d61-f401-4a76-aa16-eba4d064e851
📒 Files selected for processing (6)
ktor-server/ktor-server-plugins/ktor-server-health-check/README.mdktor-server/ktor-server-plugins/ktor-server-health-check/build.gradle.ktsktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.ktsettings.gradle.kts
✅ Files skipped from review due to trivial changes (4)
- ktor-server/ktor-server-plugins/ktor-server-health-check/README.md
- ktor-server/ktor-server-plugins/ktor-server-health-check/build.gradle.kts
- ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt
- settings.gradle.kts
…n/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…n/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
fe974f9 to
09f8558
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt (1)
81-83:⚠️ Potential issue | 🟠 MajorDo not return raw exception messages from health responses.
Line 82 forwards
cause.messageto callers, which can leak internal details from dependencies or infrastructure errors.🔒 Proposed change
} catch (cause: Exception) { - CheckResult(namedCheck.name, CheckStatus.DOWN, cause.message ?: "Health Check Failed") + CheckResult(namedCheck.name, CheckStatus.DOWN, "Health check failed") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt` around lines 81 - 83, The catch block in HealthCheck.kt that currently returns CheckResult(namedCheck.name, CheckStatus.DOWN, cause.message ?: "Health Check Failed") must not forward raw exception messages; instead return a generic message like "Health check failed" (or the existing fallback) and log the full exception separately using the module logger (so callers don't receive internal error details). Update the catch in the health check execution (the code producing CheckResult) to remove cause.message from the response, replace it with a safe constant, and add a logger.error or logger.debug call that includes the caught exception (cause) for internal diagnostics; keep CheckResult(namedCheck.name, CheckStatus.DOWN, "<generic>") as the returned value.
🧹 Nitpick comments (3)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt (2)
31-33: Validate blank endpoint paths before normalizing.Line 32 turns
""into"/", which can silently register the root path. Add an explicit non-blank guard first.🔧 Proposed change
private fun addEndpoint(path: String, block: HealthCheckBuilder.() -> Unit) { + require(path.isNotBlank()) { "Health check endpoint path must not be blank" } val normalizedPath = path.ensureLeadingSlash() require(endpoints.none { it.path == normalizedPath }) { "Health check endpoint path '$normalizedPath' is already configured" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt` around lines 31 - 33, The addEndpoint function currently normalizes an empty string into "/" via ensureLeadingSlash, allowing accidental registration of the root path; before calling ensureLeadingSlash, add an explicit non-blank guard (e.g., require(path.isNotBlank()) with a clear error message) to reject blank endpoint paths, then proceed to compute normalizedPath and the existing uniqueness check against endpoints; reference addEndpoint, ensureLeadingSlash, normalizedPath and endpoints in the change.
83-85: Reject blank check names to keep payloads actionable.Line 84 allows
check(""), producing ambiguous entries in the response JSON.🔧 Proposed change
public fun check(name: String, block: suspend () -> Boolean) { + require(name.isNotBlank()) { "Health check name must not be blank" } checks += NamedCheck(name, block) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt` around lines 83 - 85, The check(name: String, block: suspend () -> Boolean) method in HealthCheckConfig allows empty or blank names which yields ambiguous NamedCheck entries in responses; validate the name (e.g., require(name.isNotBlank()) or require(name.trim().isNotEmpty())) at the start of check and throw an IllegalArgumentException with a clear message when it fails so blank names are rejected before adding NamedCheck(name, block).ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt (1)
49-57: Optional: pre-index endpoints by path to avoid per-request linear scan.
find { it.path == path }is O(n) on every probe. A precomputed map keeps lookup O(1).♻️ Proposed change
- val endpoints = pluginConfig.endpoints.toList() + val endpointsByPath = pluginConfig.endpoints.associateBy { it.path } onCall { call -> if (call.response.isCommitted) return@onCall if (call.request.httpMethod != HttpMethod.Get) return@onCall val path = call.request.path() - val endpoint = endpoints.find { it.path == path } ?: return@onCall + val endpoint = endpointsByPath[path] ?: return@onCall🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt` around lines 49 - 57, Precompute a lookup map from path to endpoint once instead of scanning pluginConfig.endpoints per request: build a Map<String, HealthEndpoint> (e.g., endpointsByPath) from pluginConfig.endpoints during plugin initialization and then in onCall use endpointsByPath[path] to get the endpoint; replace the endpoints.find { it.path == path } lookup with this O(1) map access while preserving the existing guards (call.response.isCommitted and method check) and behavior when no endpoint is found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt`:
- Around line 81-83: The catch block in HealthCheck.kt that currently returns
CheckResult(namedCheck.name, CheckStatus.DOWN, cause.message ?: "Health Check
Failed") must not forward raw exception messages; instead return a generic
message like "Health check failed" (or the existing fallback) and log the full
exception separately using the module logger (so callers don't receive internal
error details). Update the catch in the health check execution (the code
producing CheckResult) to remove cause.message from the response, replace it
with a safe constant, and add a logger.error or logger.debug call that includes
the caught exception (cause) for internal diagnostics; keep
CheckResult(namedCheck.name, CheckStatus.DOWN, "<generic>") as the returned
value.
---
Nitpick comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt`:
- Around line 49-57: Precompute a lookup map from path to endpoint once instead
of scanning pluginConfig.endpoints per request: build a Map<String,
HealthEndpoint> (e.g., endpointsByPath) from pluginConfig.endpoints during
plugin initialization and then in onCall use endpointsByPath[path] to get the
endpoint; replace the endpoints.find { it.path == path } lookup with this O(1)
map access while preserving the existing guards (call.response.isCommitted and
method check) and behavior when no endpoint is found.
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 31-33: The addEndpoint function currently normalizes an empty
string into "/" via ensureLeadingSlash, allowing accidental registration of the
root path; before calling ensureLeadingSlash, add an explicit non-blank guard
(e.g., require(path.isNotBlank()) with a clear error message) to reject blank
endpoint paths, then proceed to compute normalizedPath and the existing
uniqueness check against endpoints; reference addEndpoint, ensureLeadingSlash,
normalizedPath and endpoints in the change.
- Around line 83-85: The check(name: String, block: suspend () -> Boolean)
method in HealthCheckConfig allows empty or blank names which yields ambiguous
NamedCheck entries in responses; validate the name (e.g.,
require(name.isNotBlank()) or require(name.trim().isNotEmpty())) at the start of
check and throw an IllegalArgumentException with a clear message when it fails
so blank names are rejected before adding NamedCheck(name, block).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b2639933-f9a5-4434-b7ff-65093ab3ac0a
📒 Files selected for processing (6)
ktor-server/ktor-server-plugins/ktor-server-health-check/README.mdktor-server/ktor-server-plugins/ktor-server-health-check/build.gradle.ktsktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.ktsettings.gradle.kts
✅ Files skipped from review due to trivial changes (4)
- ktor-server/ktor-server-plugins/ktor-server-health-check/build.gradle.kts
- settings.gradle.kts
- ktor-server/ktor-server-plugins/ktor-server-health-check/README.md
- ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt
…n/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Thank you for your contributions and for taking time to propose these improvements to Ktor. We really appreciate your enthusiasm. For the specific features you’ve suggested, there are already well-established solutions in the Ktor ecosystem:
We generally do not add functionality to Ktor itself when there are mature alternatives available. This helps us to avoid duplication and keep the core surface area focused and maintainable. Because of that, we won’t be able to move forward with these PRs unfortunately. We try to route all new feature ideas through our standard process first, before anyone invests significant time in an implementation, as mentioned in our CONTRIBUTING guide. The usual flow is:
It helps us validate the use case, align on the design, and decide whether a feature belongs in Ktor core or is better served by the wider ecosystem. If you have other feature ideas in the future, it would be great to start with an issue in YouTrack or a thread in the We indeed have an issue with the discoverability of existing solutions, and that might be the reason you weren’t able to find them. We’re working on improving this. We really don’t want to lose you as a contributor - if you’re interested, please feel free to reach out to me (@Osip Fatkullin in Slack) to discuss potential tasks from our YouTrack that you could pick up. Thank you again! 🙏 |
Add HealthCheck plugin for Kubernetes readiness and liveness probes
Summary
ktor-server-health-checkmodule providing a declarative DSL for configuring health check endpoints (readinessandliveness) with named, concurrent checksMotivation
Every production Ktor application deployed to Kubernetes needs health and readiness endpoints. Currently, developers write these manually every time. Competing frameworks (Spring Actuator, Micronaut Health, Quarkus SmallRye Health) all ship this out of the box. This plugin makes it a one-liner:
Design
ApplicationPluginwithonCallinterception -- matches configured paths before routing, so endpoints work even withoutRoutinginstalledcoroutineScope { async { ... } }, minimizing probe latency for I/O-bound checksStringBuilder, no serialization dependencycommonMain, works on JVM, Native, and JS targetsResponse Format
The plugin responds with
application/json:All healthy (HTTP 200)
{ "status": "UP", "checks": [ { "name": "database", "status": "UP" }, { "name": "redis", "status": "UP" } ] }One or more unhealthy (HTTP 503)
{ "status": "DOWN", "checks": [ { "name": "database", "status": "UP" }, { "name": "redis", "status": "DOWN", "error": "Connection refused" } ] }Files Changed
settings.gradle.ktsktor-server-health-checkmodulektor-server-health-check/build.gradle.ktsktorbuild.project.server-pluginHealthCheckConfig.kt@KtorDslconfig classes:HealthCheckConfig,HealthCheckBuilderHealthCheck.ktonCallinterception, check evaluation, JSON responseHealthCheckTest.ktREADME.md